CFE-2986: Added getdir command to cf-net, copies directory and files by SimonThalvorsen · Pull Request #6049 · cfengine/core

SimonThalvorsen · 2026-03-02T10:02:31Z

Ticket: CFE-2986
Changelog: Title

cf-net/cf-net.c

larsewi

It would be nicer if we extended cf-net get to get both files and directories, instead of having two separate sub-commands. Please investigate how rsync does this and try to do it in a similar way here.

olehermanse · 2026-03-04T00:02:37Z

It would be nicer if we extended cf-net get to get both files and directories, instead of having two separate sub-commands. Please investigate how rsync does this and try to do it in a similar way here.

@larsewi on the other hand, cf-net was written mainly as a protocol testing tool, and the commands mirror their protocol command names. If the GET protocol command in CFEngine network protocol can only GET a file, it might be a bit confusing that cf-net get is something very different.

There is also some advantage to not being too dynamic with this stuff. If you make a CLI that forces the user to specify if they are copying a file or a folder, you can prevent some annoying / confusing situations.

We could for example make a new subcommand, copy;

$ cf-net copy --file promises.cf
Copying file 1.2.3.4:/var/cfengine/masterfiles/promises.cf -> ./promises.cf

$ cf-net copy --file services/
Error: 'services/' does not look like a file, did you mean --directory?

$ cf-net copy --file services/.
Error: 'services/.' does not look like a file, did you mean --directory?

$ cf-net copy --directory services/
Copying directory 1.2.3.4:/var/cfengine/masterfiles/services/ -> ./services/

$ cf-net copy services/
Copying directory 1.2.3.4:/var/cfengine/masterfiles/services/ -> ./services/

$ cf-net copy services
Warning: Ambiguous if 'services' is a file or a directory - consider adding --file or --directory.
Copying directory 1.2.3.4:/var/cfengine/masterfiles/services/ -> ./services/

larsewi

Please break this up a little. Maybe a recursive solution would be more elegant given that it is a recursive problem.

cf-net/cf-net.c

+    char *local_dir = NULL;
+
+    int argc = 0;
+    while (args[argc] != NULL)


larsewi

There should be a recursion depth limit to prevent a potential malicious or misconfigured server with symlink loops (or deeply nested dirs) to cause stack overflow.

We should probably stick to a maximum path size for the same reasons above.

cf-net/cf-net.c

larsewi

Make sure to check return values of functions. It's OK to do best effort. I.e., we encounter an error but continue if it's safe. But in this case we need to cache the error so that we can later exit with failure. For some errors, like if we fail to create a directory, it does not make sense to try to fetch the files that should be in that directory afterwards.

cf-net/cf-net.c

larsewi

It's starting to look good. Just a few more nitpicks. Almost there!

cf-net/cf-net.c

Ticket: CFE-2986 Changelog: Title Signed-off-by: Simon Halvorsen <simon.halvorsen@northern.tech>

larsewi

It's starting to look very good

cf-net/cf-net.c

larsewi · 2026-03-18T13:39:32Z

cf-net/cf-net.c

+        }
+    }
+
+    args = &(args[optind]);


There's no check that a positional argument remains. Running cf-net getdir -o /tmp/ (no remote dir) will probably dereference NULL

cf-net/cf-net.c

larsewi · 2026-03-18T13:50:37Z

cf-net/cf-net.c

+            return -1;
+        }
+
+        if (S_ISDIR(sb.st_mode)) // Is directory


Should probably skip non-regular files. Copying symlinks may have unexpected results. Maybe a verbose log message would also be nice.

if (!S_ISREG(sb.st_mode)) { Log(LOG_LEVEL_VERBOSE, "Skipping non-regular file: %s", remote_full); continue; }

How does the cf-net get work? Does it copy symlinks?

cf-net/cf-net.c

larsewi · 2026-03-18T13:57:13Z

cf-net/cf-net.c

+    return ret;
+}
+
+static int CFNetGetDir(CFNetOptions *opts, const char *hostname, char **args)


Consider splitting this function up a bit if it makes sense

SimonThalvorsen requested a review from larsewi March 2, 2026 10:02

github-advanced-security bot found potential problems Mar 2, 2026

View reviewed changes

cf-net/cf-net.c Fixed Show fixed Hide fixed

cf-net/cf-net.c Fixed Show fixed Hide fixed

SimonThalvorsen force-pushed the CFE-2986 branch from f7cebad to 40a6c09 Compare March 2, 2026 10:40

larsewi changed the title ~~Added getdir command to cf-net, copies directory and files~~ CFE-2986: Added getdir command to cf-net, copies directory and files Mar 2, 2026

larsewi requested changes Mar 2, 2026

View reviewed changes

SimonThalvorsen force-pushed the CFE-2986 branch from 40a6c09 to 2bd2729 Compare March 4, 2026 14:58

larsewi requested changes Mar 5, 2026

View reviewed changes

cf-net/cf-net.c Outdated Show resolved Hide resolved

cf-net/cf-net.c Outdated Show resolved Hide resolved

cf-net/cf-net.c Outdated Show resolved Hide resolved

cf-net/cf-net.c Outdated Show resolved Hide resolved

cf-net/cf-net.c Outdated Show resolved Hide resolved

SimonThalvorsen force-pushed the CFE-2986 branch from 2bd2729 to e9f4cb3 Compare March 6, 2026 13:53

github-advanced-security bot found potential problems Mar 6, 2026

View reviewed changes

cf-net/cf-net.c Fixed Show fixed Hide fixed

cf-net/cf-net.c Fixed Show fixed Hide fixed

cf-net/cf-net.c Fixed Show fixed Hide fixed

cf-net/cf-net.c Fixed Show fixed Hide fixed

SimonThalvorsen force-pushed the CFE-2986 branch from e9f4cb3 to 5b55d16 Compare March 6, 2026 14:12

github-advanced-security bot found potential problems Mar 6, 2026

View reviewed changes

cf-net/cf-net.c

char *local_dir = NULL;

int argc = 0;

while (args[argc] != NULL)

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 115 lines.

SimonThalvorsen requested a review from larsewi March 9, 2026 14:42

larsewi requested changes Mar 10, 2026

View reviewed changes

SimonThalvorsen force-pushed the CFE-2986 branch 2 times, most recently from d68100f to 047b068 Compare March 12, 2026 14:29

SimonThalvorsen requested a review from larsewi March 12, 2026 14:37

larsewi requested changes Mar 13, 2026

View reviewed changes

SimonThalvorsen force-pushed the CFE-2986 branch 2 times, most recently from 62eb2df to c2b59c7 Compare March 13, 2026 15:49

larsewi requested changes Mar 16, 2026

View reviewed changes

SimonThalvorsen force-pushed the CFE-2986 branch from c2b59c7 to 38becec Compare March 16, 2026 13:54

SimonThalvorsen requested a review from larsewi March 16, 2026 13:56

github-advanced-security bot found potential problems Mar 16, 2026

View reviewed changes

cf-net/cf-net.c Fixed Show fixed Hide fixed

Added getdir command to cf-net, recursively copies directory

1b062de

Ticket: CFE-2986 Changelog: Title Signed-off-by: Simon Halvorsen <simon.halvorsen@northern.tech>

SimonThalvorsen force-pushed the CFE-2986 branch from 38becec to 1b062de Compare March 16, 2026 14:21

larsewi requested changes Mar 18, 2026

View reviewed changes

Conversation

SimonThalvorsen commented Mar 2, 2026

Uh oh!

Uh oh!

Uh oh!

larsewi left a comment

Choose a reason for hiding this comment

Uh oh!

olehermanse commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsewi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

larsewi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsewi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsewi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsewi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

larsewi Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsewi Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsewi Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

olehermanse commented Mar 4, 2026 •

edited

Loading

larsewi Mar 18, 2026 •

edited

Loading